[clang] [test] Verify that intrinsic headers don't use unreserved names #161817
+179
−0
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-clang Author: Martin Storsjö (mstorsjo) ChangesThis mirrors a similar test that libcxx does, to make sure that the libcxx headers don't use any unreserved symbols. The header for polluting with defines is based very far on the libcxx one; some parts of it could possibly be omitted, but I included most of it for completeness here. This should allow catching these issues earlier, to avoid issues like #161808 and #98478 happening again. Full diff: https://github.com/llvm/llvm-project/pull/161817.diff 8 Files Affected:
diff --git a/clang/test/Headers/arm-acle-header.c b/clang/test/Headers/arm-acle-header.c
index fea8472183c87..58fcc6648bdec 100644
--- a/clang/test/Headers/arm-acle-header.c
+++ b/clang/test/Headers/arm-acle-header.c
@@ -10,6 +10,8 @@
// RUN: %clang_cc1 -x c++ -triple arm64ec-windows -target-cpu cortex-a53 -fsyntax-only -ffreestanding -fms-extensions -fms-compatibility -fms-compatibility-version=19.11 %s
// expected-no-diagnostics
+#include "system_reserved_names.h"
+
#include <arm_acle.h>
#ifdef _MSC_VER
#include <intrin.h>
diff --git a/clang/test/Headers/arm-cde-header.c b/clang/test/Headers/arm-cde-header.c
index 1f60368ee9033..526202ab4f280 100644
--- a/clang/test/Headers/arm-cde-header.c
+++ b/clang/test/Headers/arm-cde-header.c
@@ -9,5 +9,7 @@
// Check that the headers don't conflict with each other
+#include "system_reserved_names.h"
+
#include <arm_cde.h>
#include <arm_mve.h>
diff --git a/clang/test/Headers/arm-cmse-header.c b/clang/test/Headers/arm-cmse-header.c
index 862572d8adcd9..c21c1ff595b4f 100644
--- a/clang/test/Headers/arm-cmse-header.c
+++ b/clang/test/Headers/arm-cmse-header.c
@@ -2,6 +2,8 @@
// RUN: %clang_cc1 -triple thumbv8m.base-eabi -fsyntax-only -ffreestanding -x c++ %s -verify -mcmse
// expected-no-diagnostics
+#include "system_reserved_names.h"
+
#include <arm_cmse.h>
typedef void (*callback_t)(void);
diff --git a/clang/test/Headers/arm-fp16-header.c b/clang/test/Headers/arm-fp16-header.c
index b1a87faebfe0b..e472654af24bd 100644
--- a/clang/test/Headers/arm-fp16-header.c
+++ b/clang/test/Headers/arm-fp16-header.c
@@ -18,4 +18,6 @@
// REQUIRES: aarch64-registered-target || arm-registered-target
+#include "system_reserved_names.h"
+
#include <arm_fp16.h>
diff --git a/clang/test/Headers/arm-neon-header.c b/clang/test/Headers/arm-neon-header.c
index 89bd5aaa25420..43b1b355d99f4 100644
--- a/clang/test/Headers/arm-neon-header.c
+++ b/clang/test/Headers/arm-neon-header.c
@@ -26,4 +26,6 @@
// REQUIRES: aarch64-registered-target || arm-registered-target
+#include "system_reserved_names.h"
+
#include <arm_neon.h>
diff --git a/clang/test/Headers/system_reserved_names.h b/clang/test/Headers/system_reserved_names.h
new file mode 100644
index 0000000000000..2783b2e5e4ae8
--- /dev/null
+++ b/clang/test/Headers/system_reserved_names.h
@@ -0,0 +1,161 @@
+// Test that headers are not tripped up by the surrounding code defining various
+// alphabetic macros. Also ensure that we don't swallow the definition of user
+// provided macros (in other words, ensure that we push/pop correctly everywhere).
+
+#define SYSTEM_RESERVED_NAME This name should not be used in Clang headers
+
+// libc++ does not use single-letter names as a matter of principle.
+// But Windows' own <wchar.h>, <math.h>, and <exception> use many of these
+// (at least C,E,F,I,M,N,P,S,X,Y,Z) as uglified function parameter names,
+// so don't define these on Windows.
+//
+#ifndef _WIN32
+#define _A SYSTEM_RESERVED_NAME
+#define _B SYSTEM_RESERVED_NAME
+#define _C SYSTEM_RESERVED_NAME
+#define _D SYSTEM_RESERVED_NAME
+#define _E SYSTEM_RESERVED_NAME
+#define _F SYSTEM_RESERVED_NAME
+#define _G SYSTEM_RESERVED_NAME
+#define _H SYSTEM_RESERVED_NAME
+#define _I SYSTEM_RESERVED_NAME
+#define _J SYSTEM_RESERVED_NAME
+#define _K SYSTEM_RESERVED_NAME
+#define _L SYSTEM_RESERVED_NAME
+#define _M SYSTEM_RESERVED_NAME
+#define _N SYSTEM_RESERVED_NAME
+#define _O SYSTEM_RESERVED_NAME
+#define _P SYSTEM_RESERVED_NAME
+#define _Q SYSTEM_RESERVED_NAME
+#define _R SYSTEM_RESERVED_NAME
+#define _S SYSTEM_RESERVED_NAME
+#define _T SYSTEM_RESERVED_NAME
+#define _U SYSTEM_RESERVED_NAME
+#define _V SYSTEM_RESERVED_NAME
+#define _W SYSTEM_RESERVED_NAME
+#define _X SYSTEM_RESERVED_NAME
+#define _Y SYSTEM_RESERVED_NAME
+#define _Z SYSTEM_RESERVED_NAME
+#endif
+
+// FreeBSD's <sys/types.h> uses _M
+//
+#ifdef __FreeBSD__
+# undef _M
+#endif
+
+// Test that libc++ doesn't use names that collide with FreeBSD system macros.
+// newlib and picolibc also define these macros
+#if !defined(__FreeBSD__) && !defined(_NEWLIB_VERSION)
+# define __null_sentinel SYSTEM_RESERVED_NAME
+# define __generic SYSTEM_RESERVED_NAME
+#endif
+
+// tchar.h defines these macros on Windows
+#ifndef _WIN32
+# define _UI SYSTEM_RESERVED_NAME
+# define _PUC SYSTEM_RESERVED_NAME
+# define _CPUC SYSTEM_RESERVED_NAME
+# define _PC SYSTEM_RESERVED_NAME
+# define _CRPC SYSTEM_RESERVED_NAME
+# define _CPC SYSTEM_RESERVED_NAME
+#endif
+
+// yvals.h on MINGW defines this macro
+#ifndef _WIN32
+# define _C2 SYSTEM_RESERVED_NAME
+#endif
+
+// Test that libc++ doesn't use names that collide with Win32 API macros.
+// Obviously we can only define these on non-Windows platforms.
+#ifndef _WIN32
+# define __allocator SYSTEM_RESERVED_NAME
+# define __bound SYSTEM_RESERVED_NAME
+# define __deallocate SYSTEM_RESERVED_NAME
+# define __deref SYSTEM_RESERVED_NAME
+# define __format_string SYSTEM_RESERVED_NAME
+# define __full SYSTEM_RESERVED_NAME
+# define __in SYSTEM_RESERVED_NAME
+# define __inout SYSTEM_RESERVED_NAME
+# define __nz SYSTEM_RESERVED_NAME
+# define __out SYSTEM_RESERVED_NAME
+# define __part SYSTEM_RESERVED_NAME
+# define __post SYSTEM_RESERVED_NAME
+# define __pre SYSTEM_RESERVED_NAME
+#endif
+
+// Newlib & picolibc use __input as a parameter name of a64l & l64a
+#ifndef _NEWLIB_VERSION
+# define __input SYSTEM_RESERVED_NAME
+#endif
+#define __output SYSTEM_RESERVED_NAME
+
+#define __acquire SYSTEM_RESERVED_NAME
+#define __release SYSTEM_RESERVED_NAME
+
+// Android and FreeBSD use this for __attribute__((__unused__))
+#if !defined(__FreeBSD__) && !defined(__ANDROID__)
+#define __unused SYSTEM_RESERVED_NAME
+#endif
+
+// These names are not reserved, so the user can macro-define them.
+// These are intended to find improperly _Uglified template parameters.
+#define A SYSTEM_RESERVED_NAME
+#define Arg SYSTEM_RESERVED_NAME
+#define Args SYSTEM_RESERVED_NAME
+#define As SYSTEM_RESERVED_NAME
+#define B SYSTEM_RESERVED_NAME
+#define Bs SYSTEM_RESERVED_NAME
+#define C SYSTEM_RESERVED_NAME
+#define Cp SYSTEM_RESERVED_NAME
+#define Cs SYSTEM_RESERVED_NAME
+// Windows setjmp.h contains a struct member named 'D' on ARM/AArch64.
+#ifndef _WIN32
+# define D SYSTEM_RESERVED_NAME
+#endif
+#define Dp SYSTEM_RESERVED_NAME
+#define Ds SYSTEM_RESERVED_NAME
+#define E SYSTEM_RESERVED_NAME
+#define Ep SYSTEM_RESERVED_NAME
+#define Es SYSTEM_RESERVED_NAME
+#define N SYSTEM_RESERVED_NAME
+#define Np SYSTEM_RESERVED_NAME
+#define Ns SYSTEM_RESERVED_NAME
+#define R SYSTEM_RESERVED_NAME
+#define Rp SYSTEM_RESERVED_NAME
+#define Rs SYSTEM_RESERVED_NAME
+#define T SYSTEM_RESERVED_NAME
+#define Tp SYSTEM_RESERVED_NAME
+#define Ts SYSTEM_RESERVED_NAME
+#define Type SYSTEM_RESERVED_NAME
+#define Types SYSTEM_RESERVED_NAME
+#define U SYSTEM_RESERVED_NAME
+#define Up SYSTEM_RESERVED_NAME
+#define Us SYSTEM_RESERVED_NAME
+#define V SYSTEM_RESERVED_NAME
+#define Vp SYSTEM_RESERVED_NAME
+#define Vs SYSTEM_RESERVED_NAME
+#define X SYSTEM_RESERVED_NAME
+#define Xp SYSTEM_RESERVED_NAME
+#define Xs SYSTEM_RESERVED_NAME
+
+// The classic Windows min/max macros
+#define min SYSTEM_RESERVED_NAME
+#define max SYSTEM_RESERVED_NAME
+
+// Test to make sure curses has no conflicting macros with the standard library
+#define move SYSTEM_RESERVED_NAME
+#define erase SYSTEM_RESERVED_NAME
+#define refresh SYSTEM_RESERVED_NAME
+
+// Dinkumware libc ctype.h uses these definitions
+#define _XA SYSTEM_RESERVED_NAME
+#define _XS SYSTEM_RESERVED_NAME
+#define _BB SYSTEM_RESERVED_NAME
+#define _CN SYSTEM_RESERVED_NAME
+#define _DI SYSTEM_RESERVED_NAME
+#define _LO SYSTEM_RESERVED_NAME
+#define _PU SYSTEM_RESERVED_NAME
+#define _SP SYSTEM_RESERVED_NAME
+#define _UP SYSTEM_RESERVED_NAME
+#define _XD SYSTEM_RESERVED_NAME
diff --git a/clang/test/Headers/x86-intrinsics-headers-clean.cpp b/clang/test/Headers/x86-intrinsics-headers-clean.cpp
index a19207f34ead8..0a04bcebfaece 100644
--- a/clang/test/Headers/x86-intrinsics-headers-clean.cpp
+++ b/clang/test/Headers/x86-intrinsics-headers-clean.cpp
@@ -10,4 +10,6 @@
// expected-no-diagnostics
+#include "system_reserved_names.h"
+
#include <x86intrin.h>
diff --git a/clang/test/Headers/x86-intrinsics-headers.c b/clang/test/Headers/x86-intrinsics-headers.c
index dc06cbde0f587..89a7d4dc21508 100644
--- a/clang/test/Headers/x86-intrinsics-headers.c
+++ b/clang/test/Headers/x86-intrinsics-headers.c
@@ -5,6 +5,8 @@
// XFAIL: target=arm64ec-pc-windows-msvc
// These intrinsics are not yet implemented for Arm64EC.
+#include "system_reserved_names.h"
+
#if defined(i386) || defined(__x86_64__)
#ifdef __SSE4_2__
|
|
And as intended, this test fails for now as the issue still is present in the headers. |
mstorsjo
force-pushed
the
clang-test-header-identifiers
branch
from
abf881e to
cbe22bc
Compare
|
Rebased on top of the fix now; now this is supposed to pass. The test header contains a number of references to libc++ still - I'm open to suggestions on either rewriting it, removing parts that may be less relevant here, or just keeping it as is. |
I've no problem with referring to libc++ but it might make sense to add a comment explaining that this should stay in sync with the libc++ reference file |
Yeah, some sort of such reference could be reasonable, and could make the origins and context of the checks clearer. A second issue is that this patch currently only covers the couple of x86/arm intrinsic headers that I manually added the include for; ideally we'd test all clang bundled headers with something like this. It's easy to slip in the |
This mirrors a similar test that libcxx does, to make sure that the libcxx headers don't use any unreserved symbols. The header for polluting with defines is based very far on the libcxx one; some parts of it could possibly be omitted, but I included most of it for completeness here.
mstorsjo
force-pushed
the
clang-test-header-identifiers
branch
from
cbe22bc to
80a6851
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This mirrors a similar test that libcxx does, to make sure that the libcxx headers don't use any unreserved symbols.
The header for polluting with defines is based very far on the libcxx one; some parts of it could possibly be omitted, but I included most of it for completeness here.
This should allow catching these issues earlier, to avoid issues like #161808 and #98478 happening again.